Skip to content

Flush each full Netty output buffer - #1633

Merged
jlouvel merged 2 commits into
3.0from
fix/1632-flush-full-output-buffers
Nov 3, 2026
Merged

jlouvel merged 2 commits into
3.0from
fix/1632-flush-full-output-buffers

Conversation

@jlouvel

@jlouvel jlouvel commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1632.

ChannelOutputStream, used for Netty request and response bodies, sent a full buffer with channel.write() without flushing. Netty kept those bytes until a flush() or until more than the write buffer high-water mark (64 KiB by default) was pending. So a body whose producer paused after filling one or two 32 KiB buffers held back what it had already produced until it resumed or ended. Entities copied through IoUtils.copy, such as InputRepresentation, only flush at the end, so any slowly produced stream was affected.

Full buffers are now written and flushed, so only a partially filled buffer waits for an explicit flush(), as with any buffered output stream. flush() and close() already flushed, so send() loses its flag.

Testing

  • New FullBufferFlushTestCase, over HTTP/1.1 and HTTP/2: the server writes 64 KiB and waits, without flushing, for the client to read it. It fails on the previous code (the server's wait times out) and passes with the fix.
  • Core full suite: 2397 tests (1 skipped), all passing.

🤖 Generated with Claude Code

ChannelOutputStream sent a full buffer with channel.write() without
flushing, so Netty kept it until a flush() or until more than the write
buffer high-water mark was pending. A body whose producer paused after
filling one or two buffers held back what it had produced. Full buffers
are now written and flushed, so only a partially filled buffer waits for
an explicit flush(). Fixes #1632.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Signed-off-by: Jerome Louvel <[email protected]>
@jlouvel
jlouvel requested a balanced review from Copilot November 3, 2026 15:25
@jlouvel jlouvel self-assigned this Nov 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The regression test can miss a producer timeout due to a race and does not cover a pause after the first full buffer.

2 open issues
What changed in this PR

Addresses #1632 by flushing each full Netty request or response output buffer, preventing buffered data from being held back when its producer pauses.

Changes:

  • Uses writeAndFlush() for every buffer sent and removes the send() flag.
  • Adds an HTTP/1.1 and HTTP/2 regression test for a producer that pauses after writing 64 KiB.
  • Documents the behavior in the class Javadoc and changelog.
File Description
org.restlet/​src/​test/​java/​org/​restlet/​engine/​connector/​FullBufferFlushTestCase.java Tests delivery of full response buffers while the producer waits.
org.restlet/​src/​main/​java/​org/​restlet/​engine/​netty/​ChannelOutputStream.java Flushes every sent buffer and documents the behavior.
changes.md Records the Netty buffering fix for #1632.

Review effort: Balanced

@sonarqubecloud

sonarqubecloud Bot commented Nov 3, 2026

Copy link
Copy Markdown

@jlouvel
jlouvel merged commit d0bde7f into 3.0 Nov 3, 2026
3 checks passed
@jlouvel
jlouvel deleted the fix/1632-flush-full-output-buffers branch November 3, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants